Skip to content

Fix spurious warning when using empty conflictColumns with MySQL#1028

Merged
dereuromark merged 1 commit into5.xfrom
fix/adapter-empty-not-null-conflict-columns
Feb 24, 2026
Merged

Fix spurious warning when using empty conflictColumns with MySQL#1028
dereuromark merged 1 commit into5.xfrom
fix/adapter-empty-not-null-conflict-columns

Conversation

@jamisonbryant
Copy link
Copy Markdown
Contributor

Summary

  • AbstractAdapter::getUpsertClause() checked $conflictColumns !== null to decide whether to warn MySQL users about ignored conflict columns, but BaseSeed::insertOrUpdate() requires callers to pass an array (no null default). The natural MySQL value is [], which passed the !== null check and triggered a spurious E_USER_WARNING.
  • Added && $conflictColumns !== [] to the condition, matching the pattern already used by SqliteAdapter::getUpsertClause().

Test plan

  • Added testInsertOrUpdateWithEmptyConflictColumnsDoesNotWarn to MysqlAdapterTest
  • Verified existing insertOrUpdate tests still pass for MySQL and SQLite

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a spurious MySQL warning when insertOrUpdate() is called with an empty conflictColumns array (the natural MySQL “no-op” value), and adds a regression test to ensure the behavior stays correct.

Changes:

  • Update MySQL upsert warning logic to only warn when conflictColumns is non-empty.
  • Add a MySQL adapter test that verifies insertOrUpdate(..., conflictColumns: []) does not emit E_USER_WARNING.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/Db/Adapter/AbstractAdapter.php Refines the MySQL conflictColumns warning condition to treat [] as “not provided”.
tests/TestCase/Db/Adapter/MysqlAdapterTest.php Adds regression coverage to ensure empty conflictColumns does not trigger warnings on MySQL.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jamisonbryant jamisonbryant marked this pull request as ready for review February 24, 2026 20:35
@dereuromark dereuromark merged commit 3d97a9b into 5.x Feb 24, 2026
18 checks passed
@dereuromark dereuromark deleted the fix/adapter-empty-not-null-conflict-columns branch February 24, 2026 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants